-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Return true from extend_thin_meta_device less #3648
base: master
Are you sure you want to change the base?
Return true from extend_thin_meta_device less #3648
Conversation
@@ -1117,7 +1117,10 @@ impl ThinPool { | |||
meta_growth, | |||
); | |||
|
|||
(ext.is_ok(), ext) | |||
match ext { | |||
Ok(Sectors(0)) | Err(_) => (false, ext), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this unreachable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jbaublitz You might be misreading the diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, so this is an optimization to not save the metadata if it's not extended at all? I am failing to see a case where this will ever be reached. Unreachable might be the wrong term, because it could be reached, but meta_growth
is guaranteed to be larger than 0. I don't see a case where this will be reached with the current implementation. I was originally thinking that ext
could be 0 but I don't see a case where meta_growth
would be non-zero but ext
would be zero.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me dig into do_extend and see if maybe it's rounding down. That could potentially cause the behavior you're seeing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand your argument. I think we can leave this in pending for now and work on simplifying it later.
Return true only if the method returned without error AND the amount to extend is non-zero. This is the same condition already used in extend_thin_data_device. Signed-off-by: mulhern <[email protected]>
c75bd89
to
b7aa992
Compare
alloc() return None if there is not enough space to satisfy the request. Signed-off-by: mulhern <[email protected]>
Is this still relevant or can it be closed? |
It may be. I need to take another look. |
No description provided.